fix(install): use src-layout package-dir so editable install ships a .pth#748
Merged
Conversation
….pth (not a PEP 660 finder)
The per-package `package-dir = {fbuild = "python/fbuild",
"fbuild.api" = "python/fbuild/api"}` map worked for the wheel build but
made setuptools' editable install emit a PEP 660 meta-path finder
(`__editable___fbuild_*_finder.py`) whose `MAPPING` only registered
`fbuild`. `fbuild.api` resolved at runtime via
`importlib.machinery.PathFinder` — fine for `import fbuild.api`, but
invisible to static analyzers (ty, pyright, mypy) that walk
`top_level.txt` and `.pth` files. Downstream consumers (FastLED hit this
on `from fbuild.api import SerialMonitor`) saw
`Cannot resolve imported module 'fbuild.api'` even though runtime imports
worked.
Replace the per-package map with the canonical src-layout pattern
`package-dir = {"" = "python"}`. Setuptools now emits a plain
`.pth` pointing at `python/`, which every static analyzer handles, and
the wheel-build path is unaffected.
Verified:
- Wheel (`python -m build`): byte-for-byte identical to baseline
(10,454,675 B). Same contents: `fbuild/__init__.py`,
`fbuild/_native.pyd`, `fbuild/api/__init__.py`,
`fbuild-2.2.31.data/scripts/fbuild.exe`, dist-info.
- Sdist: +2,811 B / +0.12%, entirely from `fbuild.egg-info/` relocating
to `python/fbuild.egg-info/`. Cosmetic.
- `ci/publish.py::build_wheel` is independent of setuptools — it
hand-rolls wheels from the hard-coded `PYTHON_SHIMS_DIR = ROOT /
"python"`. Smoke-tested: still finds the same 2 shims
(`fbuild/__init__.py`, `fbuild/api/__init__.py`). Release path
unaffected.
- Editable reinstall path: ~8s (FastLED → fbuild via uv sources),
within noise of baseline. No regression to the perf work in #743 /
#744.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough
ChangesSetuptools Package Directory Mapping
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This was referenced Jun 22, 2026
zackees
added a commit
that referenced
this pull request
Jun 22, 2026
Minor bump. Picks up: - #747 native fbuild as raw wheel script (drops Python launcher; fixes Windows stdout-ordering bug on COM25 303A:1001 USB Serial Device (COM25) ser=80:F1:B2:D1:DF:B1 └─ Espressif Systems / ESP32-S3 USB-CDC COM1 [Unknown] └─ (no USB identifier — Unknown endpoint) COM3 [Unknown] └─ (no USB identifier — Unknown endpoint) COM22 303A:1001 USB Serial Device (COM22) ser=D8:3B:DA:41:64:C0 └─ Espressif Systems / ESP32-S3 USB-CDC COM4 [Unknown] └─ (no USB identifier — Unknown endpoint) COM9 303A:1001 USB Serial Device (COM9) ser=8C:BF:EA:CF:87:B4 └─ Espressif Systems / ESP32-S3 USB-CDC COM20 16C0:0483 USB Serial Device (COM20) ser=15821020 └─ Van Ooijen Technische Informatica / Teensyduino Serial COM10 1FC9:0132 USB Serial Device (COM10) ser=0B03400A └─ NXP Semiconductors / LPC-Link2 CMSIS-DAP 5 USB ports, 3 non-USB ports) - #748 src-layout package-dir (static analyzers resolve fbuild.api) - #750 daemon WS handler split into reader/writer/inbound - #751 gitignore .extern-repos/ - #752 cargo fmt + extract test modules so 4 files fall under the 1000-LOC gate No public-API changes beyond what's already documented in the merged PRs above. Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The per-package
package-dir = {fbuild = "python/fbuild", "fbuild.api" = "python/fbuild/api"}map made setuptools' editable install emit a PEP 660 meta-path finder whoseMAPPINGonly registeredfbuild.fbuild.apiresolved at runtime viaimportlib.machinery.PathFinder— fine forimport fbuild.api, but invisible to static analyzers (ty, pyright, mypy) that walktop_level.txt/.pthfiles.Downstream consumers (FastLED hit this on
from fbuild.api import SerialMonitor) sawCannot resolve imported module 'fbuild.api'even though runtime imports worked. The Stop-hook lint in FastLED failed on everybash lintafter switching to an editable fbuild install.Replace the per-package map with the canonical src-layout pattern
package-dir = {"" = "python"}. Setuptools now emits a plain.pthpointing atpython/, which every static analyzer handles, and the wheel-build path is unaffected.Verification
Wheel artifact (
python -m buildof both states, cp310-cp310-win_amd64):fbuild/__init__.py,fbuild/_native.pyd,fbuild/api/__init__.py,fbuild-2.2.31.data/scripts/fbuild.exe, dist-infofbuild.egg-info/relocating topython/fbuild.egg-info/. Cosmetic.Release path (
ci/publish.py) is independent of[tool.setuptools]— it hand-rolls wheels from the hard-codedPYTHON_SHIMS_DIR = ROOT / "python". Smoke-tested: still finds the same 2 shims (fbuild/__init__.py,fbuild/api/__init__.py). SameEXTENSION_NAMES. No change.Perf (measured against the work in #743 / #744):
[tool.uv.sources]): ~8s before, ~8s after. Within noise.uv run python --version: 110-139ms. Within perf(uv): rebuild fbuild ~14x faster on no-source-change reinstall #743's baseline noise.BuildWithCargo.rununtouched. Dev-profile gate (perf(build): default to dev profile + rust-lld for ~5x faster Rust-edit rebuild #744) untouched.scripts=binary shipping (Ship fbuild as a raw native binary, drop the Python wrapper (cmd → fbuild.exe directly) #746) untouched.Test plan
python -m buildproduces byte-identical wheelpython -m buildproduces structurally-identical sdistci/publish.py::read_project_meta()+PYTHON_SHIMS_DIR.rglob('*.py')resolves correctly__editable__.fbuild-2.2.31.pthcontains<fbuild>/pythonuv run ty checkresolves bothimport fbuildandfrom fbuild.api import SerialMonitoruv run fbuild --version→fbuild 2.2.31import fbuild.apiresolves topython/fbuild/api/__init__.py🤖 Generated with Claude Code
Summary by CodeRabbit